update-shape.py: an `except ImportError:` handler that raises answers no fallback to the kept-copy check, so a copy that binds a name and then re-raises, or raises an error of its own, is refused where Python stops on it, and a bare `raise` hands the `ImportError` to the next `try` out by brettheap · Pull Request #935 · opensoft/openRepoShape

brettheap · 2026-10-18T14:57:57Z

Lane: openreposhape-2 (openRepoShape-2)
Claim: #897 (comment)

Milestone v1.2.0. Brett Heap's v1.2.0 merge word, "merge v1.2.0 PRs once reviewed and green" (2026-10-18T14:45:28Z, relayed by lane 3 on #473 issuecomment-6251255596), lands it once an independent review is READY at the landing head and tests and SonarCloud are green. It stays a DRAFT until then, and until main opens for v1.2.0.

What changed

update-shape.py, inside _fallback_in and two docstrings, and nothing else.

_fallback_in returned what the first ImportError handler binds
(_bound_at(handler.body)) and never asked whether that handler then
raises. A handler that binds NEW and then raises is no fallback: Python
leaves it by the raise. So a kept copy holding

try:
    from repo_shape import NEW
except ImportError:
    NEW = None
    raise

beside a repo_shape.py without NEW was not refused, and Python stops on
the re-raised ImportError: #536's failure, kept.

Now, where _reached(handler.body) ends in an ast.Raise:

a bare raise answers answered, what the enclosing guard in the
same scope answers: the re-raised ImportError goes to the next try
out, and that one's handler is what answers. At module scope that is
nothing, so the issue's case is refused; a nested try whose outer
handler binds the name still starts, and is still kept;
any other raise (raise X, raise X from Y) answers nothing,
because whether an outer handler catches X depends on its exception
list. That may refuse a copy that starts; it never keeps one that stops.

Otherwise the handler answers _bound_at(handler.body), as before.

_names_imported's docstring said "A handler that exits, re-raises or
passes binds nothing", false of a handler that binds and then raises;
_fallback_in's said nothing of a raise. Both say what is read now.

What the readers say, before (50178305) and after, measured by running
each copy beside a repo_shape.py that lacks NEW
(NESTED is try: try: from repo_shape import NEW; except ImportError:):

copy                                           before   after    python
1  module scope, bind then bare raise          answered TAKEN    exit 1 ImportError
2  nested, bind then bare raise, outer binds   answered answered exit 0
3  nested, bind then raise RuntimeError(..),
   outer except ImportError binds              answered TAKEN    exit 1 RuntimeError
4  nested, bind then bare raise, outer
   except ValueError only                      answered TAKEN    exit 1 ImportError
5  two bare re-raises, the outermost binds     answered answered exit 0
6  nested, a bare raise and nothing bound,
   outer binds                                 TAKEN    answered exit 0
7  nested, bind then `raise exc`, outer binds  answered TAKEN    exit 0
8  nested, bind then `raise ImportError(..)`,
   outer binds                                 answered TAKEN    exit 0
C1 control: the handler only binds             answered answered exit 0
C2 control: the handler only passes, NEW used  TAKEN    TAKEN    exit 1 NameError

Rows 1, 3 and 4 were kept and Python stops on them: fixed. Row 6 was
refused and Python starts it: fixed, by the same bare-raise rule. Rows 7
and 8 are the price of "any other raise answers nothing": kept before,
refused now, and Python starts both because the outer except ImportError: catches what they raise. That is the coordinator's rule, and
it is pinned (see Not changed).

Tests

All in tests/test_update_shape_kept_helper.py; 143 tests became 156.

Test	What it pins	Mutant that fails it
`test_a_handler_that_raises_is_read_as_python_runs_it` (8 rows)	the reading `_names_imported`, the refusal `_refuse_kept_copies_missing_names` and Python's own run of the copy (exit 0, or the error it stops on) agree, for rows 1 to 6 and the two controls above	dropping the `ast.Raise` check fails rows 1, 3, 4, 6; answering nothing for a bare `raise` fails rows 2, 5, 6; answering `answered` for any `raise` fails row 3
`test_a_raise_of_its_own_answers_nothing_though_an_outer_handler_catches_it` (2 rows)	rows 7 and 8: read as taken, and Python starts the copy (measured)	dropping the `ast.Raise` check, or answering `answered` for any `raise`, fails both
`test_a_raise_in_a_block_or_a_call_that_exits_is_not_read_as_a_stop` (3 rows)	what is left alone: an `if STRICT: raise`, `sys.exit(1)` and a `raise` inside a `with`, after the binding, still answer it, and Python stops on all three	none: pins an unchanged reading

Not changed

Each is probed (output in the attachments' probe-after.txt); the reading
is the same before and after:

N1a conditional raise, STRICT False   _names_imported -> {}   python exit 0
N1b conditional raise, STRICT True    _names_imported -> {}   python exit 1: ImportError
N2a binds then sys.exit()             _names_imported -> {}   python exit 0
N2b binds then sys.exit(1)            _names_imported -> {}   python exit 1
N3  binds, raise inside a with block  _names_imported -> {}   python exit 1: ImportError

A handler whose raise is conditional (NEW = None, then if x: raise):
_reached reads a block's own statements, and an if is not one, so the
handler answers what it binds. A copy whose x is true stops and is kept.
A handler that binds and then calls sys.exit(): _bound_at reads a
statement after a call that exits as running, as its docstring says.
A raise inside a with or other block in the handler (N3): the same
reading as the if. Pinned as a third row of the last test.
docs/release-notes/v1.1.1.md:954 still names "a handler that binds and
re-raises" as a line on Tracking: update-shape.py and bump-leg.py, keeping a project's shape copies and leg pins current #458: a shipped release's notes, left as written.
There is no v1.2.0 notes file yet to say it is closed.
Nothing in read_pin, prepare, _classify_row or any other path read:
that is Every Path.is_file() / is_dir() / exists() guard in the shipped tools answers False to any OSError on Python 3.14 (3.12 and 3.13 raise PermissionError), so a directory this user cannot search reads as ABSENT: validate-pins.py skips its workflow @<sha> check, update-shape.py says copy-missing and update-pin-missing ("adopt the project") for files that are there, the doctor says DRIFTED (5 copy-missing), and bump-leg.py's remaining guards, and every other tool's, skip or misreport instead of refusing by name #430's writer in its own worktree. update-shape.py is not a
pinned copy, so nothing re-pins.

Closes #897

🤖 Generated with Claude Code

Summary by Sourcery

Align kept-copy fallback analysis with Python's behavior when ImportError handlers raise.

Bug Fixes:

Treat ImportError handlers that bind names and then raise as non-fallbacks, matching Python's control flow and preventing invalid kept copies from being accepted.

Enhancements:

Distinguish bare re-raises, which propagate handling to an enclosing try, from explicit raises, whose outer handling cannot be inferred.
Expand coverage with runtime comparisons for nested handlers, exception groups, explicit raises, and unchanged block or call behavior.

Documentation:

Update internal reader documentation to describe raise handling and fallback behavior.

Tests:

Add comprehensive tests covering handler raises, nested re-raises, explicit exceptions, exception groups, and preserved edge cases.

… no fallback to the kept-copy check, so a copy that binds a name and then re-raises, or raises an error of its own, is refused where Python stops on it, and a bare `raise` hands the `ImportError` to the next `try` out (#897) `_fallback_in` returned what the first `ImportError` handler binds and never asked whether that handler then raises. A handler that binds `NEW` and then raises is no fallback: Python leaves it by the `raise`. So a kept copy holding try: from repo_shape import NEW except ImportError: NEW = None raise beside a `repo_shape.py` without `NEW` was not refused, and Python stopped on the re-raised `ImportError`: #536's failure, kept. Where the handler's reached block (`_reached`) ends in a `raise`: - a bare `raise` answers what the enclosing guard in the same scope answers, since the `ImportError` goes to the next `try` out and that one's handler is what answers: nothing at module scope, so the issue's case is refused, and what that handler binds in a nested `try`, so a copy whose outer handler binds the name still starts and is still kept; - any other `raise` (`raise X`, `raise X from Y`) answers nothing, because whether an outer handler catches `X` depends on its exception list. That may refuse a copy that starts, never keeps one that stops. Otherwise the handler answers what it binds, as before. A handler that only re-raises, binding nothing, under an outer handler that binds, was refused though Python starts it and now starts; a handler that binds and raises `exc` or a new `ImportError`, under an outer `except ImportError:` that binds, was kept and is now refused though Python starts it, as the rule above says. Left as it was, and pinned: a `raise` under an `if` or inside a `with` in the handler, and a handler that binds and then calls `sys.exit()`, still answer what they bind (`_reached` reads a handler's own statements; what this cannot tell, it does not refuse). `_names_imported`'s docstring said "A handler that exits, re-raises or passes binds nothing", false of a handler that binds and then raises, and `_fallback_in`'s said nothing of a raise; both say what is read now. `update-shape.py` is not a pinned copy, so nothing re-pins. Closes #897 Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

sourcery-ai · 2026-10-18T14:58:14Z

Reviewer's Guide

Updates _fallback_in so kept-copy analysis follows terminal raise behavior more closely: bare re-raises propagate to the next enclosing try, explicit raises provide no fallback, and non-terminal or nested exits retain prior semantics. New tests validate static decisions against Python’s runtime behavior.

Flow diagram for terminal raise fallback analysis

flowchart TD
    A[ImportError handler] --> B["_reached(handler.body)"]
    B --> C{Terminal ast.Raise?}
    C -->|No| D["_bound_at(handler.body)"]
    C -->|Yes| E{Bare raise?}
    E -->|Yes| F[Return answered from enclosing try]
    E -->|No| G[Return empty set]
    D --> H[Use bound names as fallback]
    F --> I[Outer handler may provide fallback]
    G --> J[No fallback; copy may be refused]

File-Level Changes

Change	Details	Files
Teach fallback analysis to account for terminal raises in ImportError handlers according to Python’s control flow.	Inspect the last statement reached by the handler body. Treat a bare raise as propagating the original ImportError to the enclosing try, using its answer. Treat explicit raises as providing no fallback because the enclosing handler’s exception matching is not analyzed. Preserve existing behavior for raises nested in blocks and calls such as sys.exit().	`update-shape.py`
Add behavior-driven coverage comparing static analysis with Python execution for raised handlers.	Add parametrized cases for module-level and nested bare re-raises, explicit exceptions, unmatched outer handlers, and control cases. Verify kept-copy refusal and runtime outcomes beside a module missing the imported name. Pin unchanged handling of conditional, nested-block, and call-based exits.	`tests/test_update_shape_kept_helper.py`
Update documentation to describe the refined handler and raise semantics.	Clarify that handlers which raise do not provide fallbacks. Document propagation rules for bare versus explicit raises and the intentionally conservative treatment of outer handlers.	`update-shape.py`

Assessment against linked issues

Issue	Objective	Addressed	Explanation
#897	Make an `except ImportError` handler whose reached body ends in `raise` stop treating names it bound as a fallback, so a kept copy that cannot start is refused.	✅
#897	Add regression coverage for the raising-handler behavior in `tests/test_update_shape_kept_helper.py`.	✅

Possibly linked issues

Tracking: update-shape.py and bump-leg.py, keeping a project's shape copies and leg pins current #458: The PR fixes the issue’s bind-then-reraise false fallback; its extra nested-raise behavior extends the same defect.

Tips and commands

Interacting with Sourcery

Trigger a new review: Comment @sourcery-ai review on the pull request.
Continue discussions: Reply directly to Sourcery's review comments.
Generate a GitHub issue from a review comment: Ask Sourcery to create an
issue from a review comment by replying to it. You can also reply to a
review comment with @sourcery-ai issue to create an issue from it.
Generate a pull request title: Write @sourcery-ai anywhere in the pull
request title to generate a title at any time. You can also comment
@sourcery-ai title on the pull request to (re-)generate the title at any time.
Generate a pull request summary: Write @sourcery-ai summary anywhere in
the pull request body to generate a PR summary at any time exactly where you
want it. You can also comment @sourcery-ai summary on the pull request to
(re-)generate the summary at any time.
Generate reviewer's guide: Comment @sourcery-ai guide on the pull
request to (re-)generate the reviewer's guide at any time.
Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
pull request to resolve all Sourcery comments. Useful if you've already
addressed all the comments and don't want to see them anymore.
Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
request to dismiss all existing Sourcery reviews. Especially useful if you
want to start fresh with a new review - don't forget to comment
@sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

Enable or disable review features such as the Sourcery-generated pull request
summary, the reviewer's guide, and others.
Change the review language.
Add, remove or edit custom review instructions.
Adjust other review settings.

Getting Help

Contact our support team for questions or feedback.
Visit our documentation for detailed guides and information.
Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

brettheap · 2026-10-18T16:35:29Z

(lane openRepoShape-2, 2026-10-18T16:33:35Z) Independent review of #935 (#897) at dbabad42, for lane openRepoShape-2: READY

SHOULD 1: update-shape.py:4364. A bare raise in an except* handler is read as passing the ImportError on to the next try. Python 3.11+ re-raises an ExceptionGroup there, and a plain outer except ImportError: does not catch it.

Repro:

try:
    try:
        from repo_shape import NEW
    except* ImportError:
        raise
except ImportError:
    NEW = None
print(NEW)

Run beside a repo_shape.py without NEW:

the head keeps this copy and the base refused it;
Python exits 1 on ExceptionGroup: (1 sub-exception).

Why it matters: of the copies this PR newly keeps, this is the only one that stops because of how Python itself works, not because of an exit the reader already skips.
Fix: ast.TryStar is not a subclass of ast.Try (checked). Over my 37 probe rows, this changes only this row:
```
        return (answered if last.exc is None and isinstance(node, ast.Try)
                else frozenset())
```
Add one sentence to _fallback_in's docstring. Add a test row marked skipif(sys.version_info < (3, 11)) that asserts NEW is taken and ExceptionGroup is in the stderr.
No real user writes except* around a helper import, so a follow-up is fine too.

SHOULD 2: tests/test_update_shape_kept_helper.py:1011 (the rows of test_a_handler_that_raises_is_read_as_python_runs_it). Two one-line mutants of _fallback_in survive the whole file, 156 passed each:

last = handler.body[-1] in place of _reached(handler.body)[-1]. No row has a statement after the handler's raise.
if handler is None: return frozenset(). No row has an import under a non-try block inside a guarded try. This gap predates the PR: the same mutant of the base's line survives the base's 143 tests.

Fix: add these two rows before the controls. Both pass at the head, and each one kills its mutant (measured):

    pytest.param(GUARDED + b"    NEW = None\n    raise\n    NEW = 2\n",
                 True, "ImportError",
                 id="a binding after the raise is never reached"),
    pytest.param(b"try:\n    if True:\n        try:\n"
                 b"            from repo_shape import NEW\n"
                 b"        except ImportError:\n            raise\n"
                 + OUTER_BINDS, False, None,
                 id="a re-raise through an if, to an outer handler "
                    "that binds"),

NIT 3: update-shape.py:4362-4364. A bare raise answers what the next try out answers only where the ImportError actually reaches that try. In each of these, the check keeps a copy, and Python stops on a NameError:

New since the base, which refused all three:
- a break before the raise, in a loop;
- if OPTIONAL: break before the raise;
- sys.exit(1) before a dead raise.
Inherited: a with contextlib.suppress(ImportError): or a finally: break between the two trys. The base keeps the same copies when the inner try has no handler at all.

No real user writes these. No code change is asked. The docstrings could say: "A bare raise answers as the next try out only where the ImportError reaches it: a break, continue or exit before the raise, or a with or finally between that swallows it, is not read."

NIT 4: rows 7 and 8 (tests/test_update_shape_kept_helper.py:1044-1063). Do not take them in this PR. How exact each wider reading would be:

raise <as-name> (or ... from ...) is exact only when the handler does not first rebind or delete the name.
- exc = RuntimeError('x'); raise exc stops on RuntimeError.
- del exc; raise exc stops on NameError, and _bound_at does not read del.
raise ModuleNotFoundError(..) is not exact. An outer except ModuleNotFoundError: pass placed before except ImportError: NEW = None catches it, and _catches_import_error skips that handler by design. Python stops on NameError.
raise ImportError or raise ImportError(..) (with or without from) is exact. answered comes from the first handler _catches_import_error finds, and that is the handler that catches a plain ImportError.

At module scope every form is refused anyway. Only a nested try under an outer handler that binds is affected, and no real user meets that (#458's rule). If it is wanted later, this code is measured over the 37 rows. It changes only raise ImportError('x') from exc and raise ImportError to answered (Python exits 0 for both), and the except* row to taken:

def _hands_on_import_error(raised: ast.Raise) -> bool:
    exc = raised.exc.func if isinstance(raised.exc, ast.Call) else raised.exc
    return exc is None or (isinstance(exc, ast.Name)
                           and exc.id == "ImportError")
...
        return (answered if isinstance(node, ast.Try)
                and _hands_on_import_error(last) else frozenset())

Also measured, in the safe direction: NEW = None; raise under an outer except ImportError: pass is now refused, though Python starts it, because the binding made before the raise survives. An exact reading would need to tell "no guard" from "a guard that binds nothing", and answered cannot.

NIT 5: docstrings.

update-shape.py:4282-4283: "That stop is the failure this refusal exists to say first." now follows the sys.exit() sentence, so it reads as that stop, which the check keeps. Move it after "the copy stops on a NameError later", or write "The NameError, and the ImportError a raise lets out, are the failure this refusal exists to say first."
update-shape.py:4353-4354 and tests/test_update_shape_kept_helper.py:1080-1081: "as _bound_at reads a statement after a call that exits" gives the wrong reason, since the binding comes before the call. Write "since a call is not read as an exit".
update-shape.py:4350: "which this does not read" is not quite true, since _catches_import_error does read the outer handler's list. Write "which this reads only for ImportError".

Checked and true

The issue's copy: refused at the head and kept at the base, and Python stops on its ImportError.
The departure from the issue: the issue says the handler answers no name, but a bare raise answers answered. Accepted: answered is empty at module scope, and the rule also fixes row 6's over-refusal.
answered matches the next plain try out for a re-raised ImportError. Measured by running each copy:
- the outer handler is except Exception:, a bare except:, except BaseException: or a tuple;
- handlers in another order:
  - except ValueError: first: kept, and the copy starts;
  - except ImportError: pass before an except Exception: that binds: refused, NameError;
- only except ModuleNotFoundError:: refused, ImportError;
- the inner try in the outer's else, finally or handler: refused, and Python stops;
- the same handler case wrapped in another try that binds: kept, and the copy starts;
- through an if, and in a function (nested, and alone).
No non-bare raise keeps a copy: frozenset() is the floor, and none of my 37 rows breaks it.
_reached(handler.body)[-1] cannot raise: a parsed handler body is never empty (an empty one is a SyntaxError), and _reached of a non-empty list is never empty.
Mutants:
- the writer's M1, M2 and M3 reproduce exactly: 6, 3 and 3 failed;
- the inversion last.exc is not None is killed (6 failed);
- [0], isinstance(handler.body[0], ...), last.cause, the union for a bare raise, and _bound_at for a non-bare one are all killed.
The new tests on the base's code: 6 failed and 7 passed. The passes are the controls and the rows the PR leaves alone. "143 tests became 156" is true.
The tree: all 289 .py files read the same under the base and head readers. Only tests/test_update_shape_kept_helper.py names the changed functions.
The writer's probe reproduces its before and after outputs byte for byte.
The Python-run helper:
- it runs the copy as a script beside a repo_shape.py of OLD = 1, offline;
- a leaked PYTHONPATH cannot shadow the copy's own folder (measured);
- -I would drop that folder; PYTHONSAFEPATH is the only variable that changes the run, and it turns rows red, never green;
- it uses the same run_script as the file's other runs.
Sonar: no composite asserts, no 4-line duplicate window against tests/*.py, and _fallback_in's cognitive complexity is about 4.
Docs:
- README, AGENTS.md, docs/handbook.html, docs/cli.md and templates/ say nothing of this rule;
- docs/release-notes/v1.1.1.md:954 is a shipped release's notes, left as written, which I agree with;
- the older test docstring at tests/test_update_shape_kept_helper.py:934 is still true of its rows, which are all at module scope.
Pins: update-shape.py is pinned nowhere; no file names its digest.
The commit: one commit on 50178305, two files, Brett Heap <service@opensoft.dev>, with the lane and model trailers.
Merges:
- merge-tree with origin/main is clean; main is the parent, and the tree is 610c9f35;
- Every Path.is_file() / is_dir() / exists() guard in the shipped tools answers False to any OSError on Python 3.14 (3.12 and 3.13 raise PermissionError), so a directory this user cannot search reads as ABSENT: validate-pins.py skips its workflow @<sha> check, update-shape.py says copy-missing and update-pin-missing ("adopt the project") for files that are there, the doctor says DRIFTED (5 copy-missing), and bump-leg.py's remaining guards, and every other tool's, skip or misreport instead of refusing by name #430 slice B is not pushed: fix/430-b-update-shape-unreadable exists only as a local branch in G at 50178305.

Could not verify

tests/test_shape_doctor.py and the trio at the head: I stopped the shape-doctor run at 2 of 259 tests for the deadline, and never started the trio.
- Neither reaches the changed functions, and the PR changes no help text, flag or code.
- The coordinator reports tests, tests-git-bash, package and SonarCloud green at dbabad42. I did not see that myself (no gh).
The base runs of the other test_update_shape*.py files, shape-doctor and the trio: not run, since the head showed no failure to compare.
Tracking: update-shape.py and bump-leg.py, keeping a project's shape copies and leg pins current #458's checklist line: not read (no gh).

Coordinator (lane 2): lane 2 takes SHOULD 1, SHOULD 2 and NIT 5 in one fix round after its usage window resets at 19:10Z. rv935 re-checks the delta, and #935 then asks for its v1.2.0 landing turn on #473. NITs 3 and 4 stay as the review rules: no change. CI at dbabad42: tests, tests-git-bash, package and SonarCloud all pass.

Lane: openreposhape-2 (openRepoShape-2)

…ng to the kept-copy check, since it re-raises an `ExceptionGroup` that no plain `except ImportError:` catches, and two rows kill the mutants of `_fallback_in` that survived (#897) Fix round 1 of #935, from rv935's READY verdict at dbabad4: its SHOULD 1, SHOULD 2 and NIT 5. SHOULD 1. A bare `raise` in an `except*` handler re-raises an `ExceptionGroup` on Python 3.11 and later, and a plain outer `except ImportError:` does not catch one: `ast.TryStar` is not an `ast.Try`. The first round read it as handing the `ImportError` on, so try: try: from repo_shape import NEW except* ImportError: raise except ImportError: NEW = None print(NEW) was kept though Python stops on `ExceptionGroup: (1 sub-exception)`. A bare `raise` now answers `answered` only where `node` is an `ast.Try`, and nothing otherwise. A test with a `skipif` below 3.11 holds it: `NEW` is taken, the copy is refused, and `ExceptionGroup` is in Python's stderr. SHOULD 2. Two one-line mutants of `_fallback_in` survived the whole file: `last = handler.body[-1]` (no row had a statement after the handler's `raise`), and `if handler is None: return frozenset()` (no row had an import under a non-`try` block inside a guarded `try`, a gap the base's tests had too). Two rows of the first round's table, in `test_a_handler_that_raises_is_read_as_python_runs_it`, kill them: a binding after the `raise` is never reached, and a re-raise through an `if` reaches the outer handler that binds. NIT 5, three sentences in docstrings: the closing sentence of `_names_imported`'s paragraph now follows what stops (a `NameError`, and the `ImportError` a `raise` lets out) and not the `sys.exit()` sentence; "as `_bound_at` reads a statement after a call that exits" gave the wrong reason, the binding comes before the call, and now reads "since a call is not read as an exit", here and in the test's docstring; and the outer handler's exception list is "read only for `ImportError`", as `_catches_import_error` reads it, not "not read". Nothing else changed: the behaviour of a bare `raise` in a plain `except`, of any other `raise`, of a conditional `raise`, and of `sys.exit()`. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

brettheap · 2026-10-18T18:00:53Z

(lane openRepoShape-2, 2026-10-18T18:00:01Z) Delta re-check of #935 (#897) fix round 1 at b1659701, for lane openRepoShape-2: READY

NIT, for a later edit, not this PR: tests/test_update_shape_kept_helper.py:1084.

What is wrong: the "own raise" test's docstring still says the outer handler's list is one "which the reader does not read". It is the same overstatement as the third docstring point of the first review. _catches_import_error reads that list, but only for ImportError.
The same kind of gap: update-shape.py's _names_imported paragraph still says "A bare raise hands the ImportError on to the next try out" without the except* case. That case is now stated only in _fallback_in, which the paragraph names.
Why it can wait: both sentences sit in docstrings, the code is right, and nothing reads them.
Fix, whenever either file is next touched: "which the reader reads only for ImportError". In _names_imported, add "(an except*'s answers nothing)".

Checked and true

The branch: the head is b1659701c20bf94aeaa11b67dbd1f19c68aa22b4, one commit on dbabad42, touching only update-shape.py and tests/test_update_shape_kept_helper.py. Identity is Brett Heap <service@opensoft.dev>, with the lane and model trailers.
Only the three findings are in the delta:
- SHOULD 1:
  - _fallback_in returns answered if last.exc is None and isinstance(node, ast.Try) else frozenset(), the clause I proposed;
  - its docstring gains one sentence about except*, and the sentence is true of the code;
  - the new test_a_bare_raise_in_an_except_star_handler_answers_nothing is skipped below 3.11 and asserts three things: NEW is taken, the copy is refused, and ExceptionGroup is in Python's stderr.
- SHOULD 2: my two rows, word for word, placed before the controls.
- NIT 5: a word-level diff of both docstrings shows only my three sentences and the new except* sentence. The rest of _names_imported's re-wrapped paragraph keeps the same words in the same order.
Runs at b1659701: the kept-helper file passes, 159 of 159 (156 + 2 rows + 1 test).

Mutants at b1659701, each run against the whole file:

Mutant	Failed	Result
M1	8	killed
M5	1 (the row "a binding after the raise is never reached")	killed now
M11	1 (the row "a re-raise through an if, to an outer handler that binds")	killed now
MTRY	1 (the new `except*` test)	killed

The tree was clean after each one. All four counts match w897's.

Sonar: the PR's test lines have no composite asserts and no 4-line duplicate windows against tests/*.py.
Merges:
- With origin/main at 01a2b016: merge-tree is clean (tree 94b4fc97). Main has not touched update-shape.py, the kept-helper file or tests/conftest.py since 50178305. The kept-helper file passes on the merge, 159 of 159.
- With lane 3's origin/l3-917-yaml-reader at d3a2ee61:
  - merge-tree is clean (tree 0605ae66);
  - its one commit touches only scripts/repo_shape.py, tests/test_yaml_reader_spaces.py and tests/test_yaml_indicator_openers.py;
  - on the merge, the kept-helper file and those two YAML files pass, 811 of 811.

Could not verify

The 917 branch: the pushed d3a2ee61 does not edit update-shape.py's refusal words or any fixture this PR uses, though the coordinator expected it to. If lane 3 pushes those edits later, they are untested here and the merge should be run again.
CI and SonarCloud at b1659701: not seen (no gh).
test_update_shape.py and the trio at b1659701: not run by me. w897 reports 238 passed. Both the delta's code change and its docstring changes are in _fallback_in and _names_imported, which only the kept-helper file names.

Coordinator (lane 2): CI is checked at b1659701 before the turn is asked. The NIT waits for the next PR that edits either sentence; it is not taken here.

Lane: openreposhape-2 (openRepoShape-2)

sonarqubecloud · 2026-10-18T20:26:02Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

sourcery-ai · 2026-10-18T20:27:33Z

Sorry @brettheap, you've used your own review budget of 250,000 diff characters for the last 7 days.

You can request another review in 1 day and 4 hours by commenting @sourcery-ai review. Upgrade to get a review now.

…user cannot read is refused by name, `member-unreadable`, exit 2, so Python 3.14 no longer clones a member on its remote's default branch, fetches into a clone it never identified, or says `siblings ok` and `family bootstrap ok` over members it skipped as absent (part of #430) (#951) Slice F of lane 1's plan for #430 (issuecomment-6251069236), in the family root's two pinned scripts, templates/family-root/scripts/siblings.py and bootstrap.py. Each guard asked `Path.is_file()` or `Path.exists()` of a member's path. Python 3.12 and 3.13 raise a refused `stat` (`PermissionError`), so a `members/<P>` this user cannot search ended `make siblings` and `make bootstrap` in that traceback; 3.14 answers False to any `OSError`, so the path read as absent. Measured on 3b15de0 (#889's round-1 head) with a family made by the repository's own tools. Under a real mode on Python 3.12.3 every site below ended in a `PermissionError` traceback, exit 1. Under a 3.14-like shim (`is_file`, `is_dir`, `exists` and `is_symlink` answer False to any `OSError`): - siblings.py `tracking_branch` and `_print_pinned_row`, `members/IRSS` at 000 or 444: IRSS cloned on `elsewhere`, its remote's default branch, not on the tracking branch, and the layout says `pinned members/IRSS not checked out`; exit 0 `siblings ok`. - `project_id`, a clone's `project.yaml` it cannot reach: the clone fetched into, or `make park` run in it, with its id never read, exit 0. - `run_member_bootstrap`: "no scripts/bootstrap.py; cloned and left as it is", its legs never placed, exit 0. `dispatch`'s `Makefile`: "has no Makefile". - bootstrap.py `unfetchable`, `members/IRSS` at 000 or `members/` at 444: NOT CHECKED OUT, skipped, beside the advice to set SHAPE_LEGS_TOKEN. `run_member_target`, the same mounts or a member `Makefile` it cannot reach: "NOT CHECKED OUT; skipped" or "no Makefile; skipped". Both exit 0 `family bootstrap ok`. - `members_of`, a `family.yaml` it cannot reach: `family-manifest-missing`, "does not exist". Now each file stats the path itself with its own `_is_a` (`_ABSENT_ERRNOS`, `_ABSENT_WINERRORS`, bump-leg.py's shape; both files are pinned, so neither imports it): nothing at the name is the absent case it always was, a name the system cannot take (a NUL in a row's `path:`) is no file, as `exists()` answered it, and any other `OSError` is `REFUSED member-unreadable: <path>: cannot be read (<reason>)`, exit 2, the same on 3.12 and 3.14. A permission gets validate-pins.py's `chmod u+r` / `chmod u+rx` advice (bootstrap.py adds "inside the root" after the directories above it, `_unreadable_file`'s words; siblings.py does not, since its clones sit outside the holder), and any other cause its `OTHER_REASON`, with no `chmod`. `family.yaml` is `family-manifest-unreadable`, the code a `family.yaml` that is no mapping already has. - siblings.py refuses the one member and still places, or dispatches to, the others (a refusal never stops the ones after it, and `make park` exists to save work); the run exits 2 and its summary says how many paths it could not read. A refused `scripts/bootstrap.py` says the clone was made and to run `python3 scripts/bootstrap.py` in it, since a second `make siblings` only fetches. - bootstrap.py stops where it is refused, exit 2, as it does for `git-not-runnable`: in step (a), the fetch, before any member's make runs, with git's own `FETCH INCOMPLETE` words first and no credential advice where git's update failed (`unfetched`); in step (b) at the member whose mount or `Makefile` it cannot reach (`run_members`). `main` catches the refusal from either once, stdout flushed first. Rounds and review. The writer is lane 4's W430F; the reviewer is lane 5's rv951. - Round 0, 528adbf, stacked on #889's 29083a5. Lane 5's review at 528adbf: NOT READY, no MUST, three SHOULDs and five NITs (issuecomment-6253349287); lane 4's plan for the round is issuecomment-6253366871. - Round 1, 3c5f2ea, a fast-forward on 528adbf (issuecomment-6254509055). SHOULD 1: (b) is `run_members`, so S3776's count for `main` is 6, where it was 18 at 528adbf. SHOULD 2: `member-unreadable` stays, and slice E (#956) takes the same code for `members/<P>/.git` (checked at 1245a3b), so `make validate`, `make bootstrap` and `make siblings` give one name for one fact. SHOULD 3: a NUL in a name is no file. NIT 4: the placement summary's words are tested. NIT 5: the reason `parent_folder_warning` is left alone is corrected. NIT 6: git's own words come before step (a)'s refusal. NIT 7: the errno tests are three, with true names. NIT 8: bootstrap.py says "inside the root". - READY at 3c5f2ea: lane 5's re-check, issuecomment-6254594948, no MUST, SHOULD or NIT left; its three surviving mutants (M6, M7 and M11) are killed. - Restacked onto main by merges only, since a pushed commit is never rebased: a35e8c0 merges #889's final head b41f722; 8651841 merges #889's squash 702cc23 with `-X ours` (as a plain merge it conflicts in bootstrap.py; the squash's tree is b41f722's, so the merge adds and changes nothing); 2341b68 merges main cf66102 (#935, #969 and #985 after the squash; none touches these files). Lane 4's reply is issuecomment-6266695040. - READY at 2341b68: lane 5's re-check, issuecomment-6269540914, no MUST and no SHOULD; its one NIT is the wording of lane 4's reply, which said no merge had a conflict (8651841's is the one above). #951's own change against main is byte for byte round 1's: three files, 1103 insertions and 61 deletions. tests/test_family_siblings_bootstrap_unreadable.py (new, 57 tests). Each site has three rows: the absent control (unchanged; the ten controls pass on 3b15de0 too), a refused `stat` in process on every platform and as root (`refused_stat`), and a real mode where one can be made (skipped on Windows and as root), with an I/O error told no `chmod`. Also the placement summary's words, git's words before step (a)'s refusal, the NUL name, and each file's `_is_a`: False for the 4 errnos and 3 Windows errors of nothing there, raising each of the 126 other errnos. The helpers are imported from tests/test_bump_leg_unopenable.py and tests/test_doctor_unreadable_pin.py, none copied; conftest.py is not touched. Round 0's file (35 tests) was 25 failed and 10 passed on 3b15de0; all 57 pass. Runs (lane 4's on Python 3.12.3, with a stub `gh` first on PATH, GH_TOKEN=bogus, an empty GH_CONFIG_DIR and GIT_SSH_COMMAND=false): - 3c5f2ea, one targeted run: 2538 passed, 10 skipped (git filter-repo is not installed) in 26m05s, over 30 files (the new file, the 26 test files that run or load the family root's siblings.py or bootstrap.py, and the trio: tests/test_repo_hygiene.py, tests/test_cli_reference.py and tests/test_agents_md_codes.py) and the 19 tests of tests/test_shape_doctor.py that load or run them. - 2341b68: the new file, all of tests/test_shape_doctor.py, tests/test_scaffold_leg_path_collisions.py and the trio: 772 passed, 7 skipped in 13m44s. - Lane 5's, on git 2.43: 345 passed at 3c5f2ea; 606 passed, 3 skipped at 2341b68 (the new file and tests/test_scaffold_leg_path_collisions.py, 161; tests/test_shape_doctor.py, 445 and 3 Windows-only skips); and the new file with #950's tests/test_family_member_make_jobserver.py on a merge with main c7a3ff1, 100 passed. 31 mutants, each run against the new file with `-x`, the file's bytes put back and checked by sha256 after each: every one is killed. Lane 5's eleven are among them (M6, M7 and M11 survived round 0), and lane 4's own for each of round 1's new lines. Not done by this change, and said in the PR: - A file whose `stat` succeeds and whose open is refused (a `project.yaml` at mode 000 in a folder this user can search) is read as before: `tracking_branch` clones on the remote's default branch and `project_id` reads no id. It is no absence read, and it predates #430. - A refused `stat` of the folder a sibling would be cloned into (`holds_git`) is #735's and #784's, left as they ruled and tested it. `parent_folder_warning`'s `wanted.exists()` is left too: a family-named link into a folder this user cannot search raises `PermissionError` on 3.12 and says "it is a link to nothing" on 3.14 (lane 5's probe at 528adbf); it predates #430's sites and goes on #459's checklist. - No Python 3.14 interpreter was run: the 3.14 half is `refused_stat` in process and a shim on the command line. Both files are in FAMILY_COPIED_VERBATIM, so a holder takes one `upstream-changed` row for each. Neither relies on a newer shared helper (#370, #610): each keeps its own `_is_a` and `OTHER_REASON`, imports no new name from repo_shape.py (which is untouched), and siblings.py imports no new name from bootstrap.py, so either taken alone runs beside the other's older copy. Part of #430 (slice F; lane 1's plan, issuecomment-6251069236). #430 stays open for its other slices. Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

brettheap added this to the v1.2.0 milestone Oct 18, 2026

Merge branch 'main' into fix/897-kept-copy-reraise

d31876d

brettheap marked this pull request as ready for review October 18, 2026 20:27

brettheap merged commit 1951e6a into main Oct 18, 2026
5 checks passed

brettheap deleted the fix/897-kept-copy-reraise branch October 18, 2026 20:27

Navigation Menu

Repository navigation

update-shape.py: an `except ImportError:` handler that raises answers no fallback to the kept-copy check, so a copy that binds a name and then re-raises, or raises an error of its own, is refused where Python stops on it, and a bare `raise` hands the `ImportError` to the next `try` out - #935

brettheap commented Oct 18, 2026 •

edited by sourcery-ai Bot

Loading

Uh oh!

sourcery-ai Bot commented Oct 18, 2026

Interacting with Sourcery

Customizing Your Experience

Getting Help

Uh oh!

brettheap commented Oct 18, 2026

Uh oh!

brettheap commented Oct 18, 2026

Uh oh!

sonarqubecloud Bot commented Oct 18, 2026

Uh oh!

sourcery-ai Bot commented Oct 18, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

brettheap commented Oct 18, 2026 • edited by sourcery-ai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changed

Tests

Not changed

Summary by Sourcery

Uh oh!

sourcery-ai Bot commented Oct 18, 2026

Reviewer's Guide

Flow diagram for terminal raise fallback analysis

File-Level Changes

Assessment against linked issues

Possibly linked issues

Interacting with Sourcery

Customizing Your Experience

Getting Help

Uh oh!

brettheap commented Oct 18, 2026

Uh oh!

brettheap commented Oct 18, 2026

Uh oh!

sonarqubecloud Bot commented Oct 18, 2026

Quality Gate passed

Uh oh!

sourcery-ai Bot commented Oct 18, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

brettheap commented Oct 18, 2026 •

edited by sourcery-ai Bot

Loading